tests/request/auth-mode-switch.spec.ts (3)

33-37: ⚖️ Poor tradeoff

readField is coupled to the CodeMirror v5 DOM API — worth a data-testid fallback.

return editor.evaluate((el: any) => (el as any).CodeMirror?.getValue() ?? '');

el.CodeMirror is a CM5-specific instance property. If Bruno ever upgrades to CodeMirror 6 (which attaches its state differently), this evaluates to '' for every assertion, silently turning all readField assertions into vacuous passes. Wiring a data-testid attribute to the underlying <textarea> that CM5 maintains in sync, or an aria-label, would make readField CM-version-agnostic.

As per coding guidelines: "Add data-testid to testable elements for Playwright."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/request/auth-mode-switch.spec.ts` around lines 33 - 37, readField
currently relies solely on the CodeMirror v5 DOM API (reading
el.CodeMirror.getValue()), which will silently break if the editor is upgraded;
update readField to first try the existing CodeMirror path and then fall back to
reading the underlying <textarea> value via a test id or accessible label (e.g.
query for textarea[data-testid="${labelText}"] or
textarea[aria-label="${labelText}"] and return its .value) so assertions remain
valid across CM versions; also ensure the component under test includes a stable
data-testid on the editor textarea so fieldEditor/readField can locate it.

---

84-124: ⚡ Quick win

Collection and Folder tests only cover the happy path — consider adding parity steps with the Request test.

The Request test (lines 44–82) includes 5 well-structured steps:

1. Save Bearer credentials
2. Bearer → Basic → Bearer restores token ✓ (the main fix)
3. Switching to a non-saved mode shows empty fields
4. Switching to a third unrelated mode also leaves fields empty
5. Multiple round-trips to the saved mode still restore correctly

The Collection (lines 84–101) and Folder (lines 104–124) tests only cover step 2. If a bug were introduced that only affects Collection- or Folder-level auth mode switching (e.g., the updateCollectionAuth / updateFolderAuth reducers that clear auth), steps 3–5 would silently pass in the Collection/Folder paths. Adding at least step 3 (non-saved mode shows empty fields) would give meaningful regression coverage for the collection/folder reducers independently.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/request/auth-mode-switch.spec.ts` around lines 84 - 124, Add the
missing parity check that switching to a non-saved auth mode clears fields:
after the "Save Bearer" step in both the Collection and Folder tests (identify
blocks that call createCollection/createFolder, selectAuthMode, typeIntoField,
and click Save) add a step that selects a different mode that was never saved
(e.g., selectAuthMode(page, 'API Key') or 'Basic Auth' as appropriate) and
assert using readField(...) that the relevant fields are empty; then switch back
to 'Bearer Token' and assert the saved token is restored with expect.poll(() =>
readField(page, 'Token')).toBe('collection-bearer-token' /
'folder-bearer-token') to mirror the Request test's non-saved-mode coverage.

---

16-21: 💤 Low value

Static analysis flag on new RegExp(...) is a false positive here.

labelText is always passed as a static string literal ('Token', 'Username', 'Password') with no regex metacharacters, so the ReDoS warning is inapplicable. The exact-boundary regex (^...$) is the right approach to avoid partial-text false matches on labels like "Token" vs "Bearer Token."

If you want to silence the linter cleanly:

✨ Suggested fix

-    .filter({ hasText: new RegExp(`^${labelText}$`) })
+    .filter({ hasText: labelText, exact: true })

Playwright's filter({ hasText: ..., exact: true }) does case-sensitive exact string matching, removing the regex entirely.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/request/auth-mode-switch.spec.ts` around lines 16 - 21, The linter
flags the use of new RegExp in the fieldEditor helper; replace the regex-based
match with Playwright's exact string match to silence the false positive and
keep exact-boundary behavior: update the fieldEditor function (named
fieldEditor) to use filter({ hasText: labelText, exact: true }) instead of
filter({ hasText: new RegExp(`^${labelText}$`) }) so it performs case-sensitive
exact matching for labels like "Token" vs "Bearer Token".

tests/utils/page/actions.ts (1)

1248-1252: 💤 Low value

Consider removing .first() if only one editor is expected.

The readField helper uses .first() at line 1249, which suggests multiple editors might match the selector. If only one editor per label is expected in the auth forms, calling .first() might mask issues if the selector accidentally matches multiple elements. However, if this is defensive coding for edge cases, it's acceptable.

♻️ Alternative: drop `.first()` if ambiguity isn't expected

-const readField = async (page: Page, labelText: string): Promise<string> => {
-  const editor = fieldEditor(page, labelText).first();
-  await editor.waitFor({ state: 'visible' });
-  return editor.evaluate((el: any) => (el as any).CodeMirror?.getValue() ?? '');
-};
+const readField = async (page: Page, labelText: string): Promise<string> => {
+  const editor = fieldEditor(page, labelText);
+  await editor.waitFor({ state: 'visible' });
+  return editor.evaluate((el: any) => (el as any).CodeMirror?.getValue() ?? '');
+};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/utils/page/actions.ts` around lines 1248 - 1252, The helper readField
currently calls .first() on the locator returned by fieldEditor(page,
labelText), which can hide selector collisions; remove the .first() call so
readField uses the locator returned by fieldEditor(page, labelText) directly,
update the subsequent awaits/evaluate to operate on that locator (i.e., await
fieldEditor(page, labelText).waitFor(...) and then evaluate on the same
locator), and optionally add an explicit uniqueness assertion (e.g., check
locator.count() === 1 or use an expect-to-have-count assertion) if you want to
fail fast when multiple editors are matched.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes